[TSVB] Enable `dual mode`, support index patterns and strings #92812

alexwizp · 2021-02-25T13:33:46Z

Closes: #91367, Closes: #93751, Closes: #80849

Release notes

Adding an additional mode for selecting the pattern index for TSVB. Now, in addition to entering the index in string form, the user can select the desired index using the drop-down list.

The mode is switched by pressing the "Gear" button:

Summary

Describe the feature:

TSVB right now doesn't work entirely with index patterns. Specifically, it allows the user to enter a string (which can be the index pattern title, an index, multiple indices, wildcards etc).

This has served us well so far but we have identified some cases that is not going to work well in the future. For example:

we will have to support runtime fields with a hacky way
if the index patterns work with non-unique pattern lists in the future, it will be impossible for TSVB to identify which index pattern the user wants to use (Imagine that the user has created 2 index patterns with the same pattern list but only one of them supports runtime fields).

For all the above we decided to enable a dual mode in TSVB. This means that we will keep the old implementation, meaning that we will be backwards compatible and the user will be able to create a visualization by giving a string but we will also give the user the ability to create a visualization from the index pattern.

alexwizp · 2021-03-01T07:11:59Z

@elasticmachine merge upstream

stratoula · 2021-03-01T13:55:13Z

@elastic/kibana-design can you help us with that? Our goal here is to have two ways to configure a TSVB visualizations. The old one with the input string and the new one with a combobox. We want to highlight more the new implementation as the old one is going to be deprecated in the future. Aliaksei has made a first attempt but it would help us a lot to also have your feedback!

ryankeairns · 2021-03-01T19:45:39Z

I'll take a look from the design side.

ryankeairns

In general, this feels like a good approach that needs just a couple of adjustments.

While I appreciate the desire to push the new way versus the old way, the warning icon feels a bit too distracting (some may still prefer the old way, for reasons, and would be nagged by this icon).

Since we are already defaulting to the new way, I would suggest adding the description (text from the warning callout) to the settings popover in context with the switch. To clarify, I don't think we need the actual callout container, rather just move the message from there to this popover.

A pattern to follow would be the KQL popover on the filter/search bar. Add a panel title, explanatory text, and the switch below.

For the switch itself, in my opinion, the title above is not necessary if we use a more descriptive label like this:

Let me know if you have questions.

...pe_timeseries/public/application/components/lib/index_pattern_select/switch_mode_popover.tsx

alexwizp · 2021-03-02T12:37:52Z

@ryankeairns thank you for your review, I've updated UI, what do you think about it now

ryankeairns · 2021-03-02T16:16:02Z

@alexwizp thank you for the changes!

The callout in the popover is not a pattern we've used elsewhere and is not one I'd like to set. Further, adding a separate button/action in this popover seems like something we can avoid.

What do you think about simply clearing out the input if you toggle the switch? That would avoid us having to handle this messaging altogether and, in my opinion, seems a reasonable expectation if a user is setting a new index type.

The current callouts strike me moreso as validation messages. If there are cases where these still arise (say we clear the text, close the popover, and then they type an unidentified index), then we can present this copy as validation/error text below the input. Thoughts?

alexwizp · 2021-03-02T19:12:38Z

@ryankeairns I don't have a clear opinion on this. Yes, we can remove callouts. But on the other hand, it allows you to quickly navigate to the pattern index creation page ...

@stratoula @timroes maybe you have some ideas?

alexwizp · 2021-03-03T06:14:44Z

@elasticmachine merge upstream

alexwizp · 2021-03-03T14:16:44Z

@ryankeairns I removed all callouts. Now it's just a popover with text and switch like we have for kql.

alexwizp · 2021-03-25T15:08:06Z

@ryankeairns @stratoula what if we remove all related to 'Switch Mode'? Keep only gear icon with exciting popover. From the user's perspective, it will be a little more difficult to navigate to the creation page of the index pattern ...

ryankeairns · 2021-03-25T16:06:54Z

@ryankeairns @stratoula what if we remove all related to 'Switch Mode'? Keep only gear icon with exciting popover. From the user's perspective, it will be a little more difficult to navigate to the creation page of the index pattern ...

I would prefer that, yes.
We could simply replace the 'Switch mode' link with a 'Create index pattern' link. Perhaps that's a good, small compromise here. Thanks Alexey :)

gchaps · 2021-03-25T16:14:25Z

+1 on 'Create index pattern'

crob611 · 2021-03-25T18:15:37Z

@alexwizp @stratoula In regards to the dashboard reporting references, it should currently work as long as the embeddable factory has an extract method that properly extracts the references. Let me know if that isn't working and I'll be happy to dig in and figure out what's going on.

stratoula · 2021-03-26T07:33:18Z

@ryankeairns thank you a lot for the feedback. I agree, let's change the text to 'Create index pattern' link and remove the popover. It def makes more sense. Just to have the whole picture here, we are going to remove on 8.0 the option to create viz with the input text but the old visualizations that have been created that way will work. We want the users to prefer the combobox option.

@alexwizp thank you for the fixes. It seems that the dashboard references should work, so let's address it too.

VladLasitsa · 2021-03-26T12:30:16Z

I found out that when we use old way by using string as index pattern we don't get fields for time field. On master it works fine. I think we should keep behavior like in master and getting list of time fields for index pattern when we use old implementation.

Already fixed

VladLasitsa

LGTM, did local tests

alexwizp · 2021-03-26T13:49:57Z

@ryankeairns

ryankeairns

Looks great, thanks for handling all the feedback!

stratoula · 2021-03-29T05:39:55Z

@elasticmachine merge upstream

stratoula

Code LGTM, thanx @alexwizp for all the changes, this is a great PR!

stratoula · 2021-03-29T07:16:02Z

@alexwizp can you also add a brief release notes section?

stratoula · 2021-03-29T08:17:54Z

@elasticmachine run elasticsearch-ci/docs

…c#92812) * [TSVB] Enable `dual mode`, support index patterns and strings * modify UI * add migration script * refactoring * fix CI * prefill the index pattern name * modify UI * modify UI * update UI * fix functional test * some work * remove callouts * fix rollup test * update UI * fix typo * add some unit tests * add functional test * fix CI * correct labels * fix ci group 12 * cleanup interface * fix CI * cleanup API * fix some of PR comments * move index patterns into so references * remove wrong logic * fix JEST * fix some ui issues * update sample data * indexPatternObject -> indexPatternValue * fix comments * I have a dashboard with two TSVB viz. One with the default (haven't applied it to the combobox) and one with the logs. The filter contains fields only from the logs index pattern * When I am on the string mode and try to write my index, sometimes some of the chars are not added or they are deleted while typing, something with the denounce maybe? * fix merge conflicts * Does this PR also supports runtime fields? I created one from the editor and I see that I can select it * fix UI issue * If I create a viz with the string mode and a wildcard e.g. kibana_sample*, the index patterns are not communicated correctly to the dashboard. * fix import/export refs for dashboard * remove MigrationPopover Co-authored-by: Kibana Machine <[email protected]>

#95631) * [TSVB] Enable `dual mode`, support index patterns and strings * modify UI * add migration script * refactoring * fix CI * prefill the index pattern name * modify UI * modify UI * update UI * fix functional test * some work * remove callouts * fix rollup test * update UI * fix typo * add some unit tests * add functional test * fix CI * correct labels * fix ci group 12 * cleanup interface * fix CI * cleanup API * fix some of PR comments * move index patterns into so references * remove wrong logic * fix JEST * fix some ui issues * update sample data * indexPatternObject -> indexPatternValue * fix comments * I have a dashboard with two TSVB viz. One with the default (haven't applied it to the combobox) and one with the logs. The filter contains fields only from the logs index pattern * When I am on the string mode and try to write my index, sometimes some of the chars are not added or they are deleted while typing, something with the denounce maybe? * fix merge conflicts * Does this PR also supports runtime fields? I created one from the editor and I see that I can select it * fix UI issue * If I create a viz with the string mode and a wildcard e.g. kibana_sample*, the index patterns are not communicated correctly to the dashboard. * fix import/export refs for dashboard * remove MigrationPopover Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-03-29T21:04:32Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`visTypeTimeseries`	477	485	+8
`visualizations`	140	143	+3
total			+11

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`indexPatternManagement`	554.2KB	554.3KB	+84.0B
`visTypeTimeseries`	1.6MB	1.6MB	+11.1KB
total			+11.2KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`visTypeTimeseries`	124.4KB	126.8KB	+2.5KB
`visualizations`	107.4KB	109.4KB	+2.0KB
total			+4.5KB

History

💚 Build #115688 succeeded 4d5d75b
💚 Build #115469 succeeded 3416a94
💚 Build #115439 succeeded 9ddf39e
💚 Build #115194 succeeded 9d7c237
💔 Build #115167 failed 59d3e7d29338d960c9fa5c9db82364d44a6cb6b7

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @alexwizp

alexwizp added 3 commits February 25, 2021 16:19

[TSVB] Enable dual mode, support index patterns and strings

6eeeddb

modify UI

9218ccc

add migration script

f8cc389

alexwizp force-pushed the dual_mode_tsvb_2 branch from b2882cc to 0111e0f Compare March 1, 2021 09:57

refactoring

725d5e8

alexwizp force-pushed the dual_mode_tsvb_2 branch from 0111e0f to 725d5e8 Compare March 1, 2021 10:23

alexwizp added 3 commits March 1, 2021 15:25

fix CI

0f9d8ad

prefill the index pattern name

6fcdda5

modify UI

8a9113d

Merge remote-tracking branch 'upstream/master' into dual_mode_tsvb_2

81110ac

ryankeairns requested changes Mar 1, 2021

View reviewed changes

...pe_timeseries/public/application/components/lib/index_pattern_select/switch_mode_popover.tsx Outdated Show resolved Hide resolved

alexwizp added 3 commits March 2, 2021 14:11

modify UI

eea56c6

Merge remote-tracking branch 'upstream/master' into dual_mode_tsvb_2

6993121

update UI

5046545

fix functional test

c41765d

kibanamachine and others added 3 commits March 3, 2021 01:14

Merge branch 'master' into dual_mode_tsvb_2

7cb454c

some work

47fc806

remove callouts

fd0d2a1

alexwizp force-pushed the dual_mode_tsvb_2 branch from 439998f to fd0d2a1 Compare March 3, 2021 12:55

fix rollup test

79343e0

elastic deleted a comment from kibanamachine Mar 3, 2021

fix import/export refs for dashboard

9ddf39e

alexwizp added 2 commits March 26, 2021 16:20

remove MigrationPopover

f882f11

Merge remote-tracking branch 'upstream/master' into dual_mode_tsvb_2

3416a94

VladLasitsa approved these changes Mar 26, 2021

View reviewed changes

stratoula added release_note:feature Makes this part of the condensed release notes and removed release_note:enhancement labels Mar 26, 2021

ryankeairns approved these changes Mar 26, 2021

View reviewed changes

Merge branch 'master' into dual_mode_tsvb_2

4d5d75b

stratoula approved these changes Mar 29, 2021

View reviewed changes

alexwizp enabled auto-merge (squash) March 29, 2021 08:18

alexwizp merged commit 4342f26 into elastic:master Mar 29, 2021

alexwizp mentioned this pull request Mar 29, 2021

[7.x] [TSVB] Enable dual mode, support index patterns and strings (#92812) #95631

Merged

alexwizp mentioned this pull request Apr 6, 2021

[TSVB] [Regression] Fix Top Hit / Filter Ratio aggregations #96288

Merged

This was referenced Apr 12, 2021

[TSVB] Index patterns are incorrectly cached #96901

Closed

[TSVB] Fix per-request caching of index patterns #97043

Merged

alexwizp mentioned this pull request Apr 16, 2021

[TSVB] Improve dashboard performance #97371

Closed

KOTungseth mentioned this pull request May 6, 2021

[DOCS] Adds dual mode to TSVB #99493

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TSVB] Enable `dual mode`, support index patterns and strings #92812

[TSVB] Enable `dual mode`, support index patterns and strings #92812

alexwizp commented Feb 25, 2021 •

edited

Loading

alexwizp commented Mar 1, 2021

stratoula commented Mar 1, 2021

ryankeairns commented Mar 1, 2021

ryankeairns left a comment •

edited

Loading

alexwizp commented Mar 2, 2021

ryankeairns commented Mar 2, 2021

alexwizp commented Mar 2, 2021

alexwizp commented Mar 3, 2021

alexwizp commented Mar 3, 2021

alexwizp commented Mar 25, 2021 •

edited

Loading

ryankeairns commented Mar 25, 2021

gchaps commented Mar 25, 2021

crob611 commented Mar 25, 2021

stratoula commented Mar 26, 2021 •

edited

Loading

VladLasitsa commented Mar 26, 2021 •

edited

Loading

VladLasitsa left a comment

alexwizp commented Mar 26, 2021

ryankeairns left a comment

stratoula commented Mar 29, 2021

stratoula left a comment

stratoula commented Mar 29, 2021

stratoula commented Mar 29, 2021

kibanamachine commented Mar 29, 2021

[TSVB] Enable dual mode, support index patterns and strings #92812

[TSVB] Enable dual mode, support index patterns and strings #92812

Conversation

alexwizp commented Feb 25, 2021 • edited Loading

Release notes

Summary

Describe the feature:

alexwizp commented Mar 1, 2021

stratoula commented Mar 1, 2021

ryankeairns commented Mar 1, 2021

ryankeairns left a comment • edited Loading

Choose a reason for hiding this comment

alexwizp commented Mar 2, 2021

ryankeairns commented Mar 2, 2021

alexwizp commented Mar 2, 2021

alexwizp commented Mar 3, 2021

alexwizp commented Mar 3, 2021

alexwizp commented Mar 25, 2021 • edited Loading

ryankeairns commented Mar 25, 2021

gchaps commented Mar 25, 2021

crob611 commented Mar 25, 2021

stratoula commented Mar 26, 2021 • edited Loading

VladLasitsa commented Mar 26, 2021 • edited Loading

VladLasitsa left a comment

Choose a reason for hiding this comment

alexwizp commented Mar 26, 2021

ryankeairns left a comment

Choose a reason for hiding this comment

stratoula commented Mar 29, 2021

stratoula left a comment

Choose a reason for hiding this comment

stratoula commented Mar 29, 2021

stratoula commented Mar 29, 2021

kibanamachine commented Mar 29, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

[TSVB] Enable `dual mode`, support index patterns and strings #92812

[TSVB] Enable `dual mode`, support index patterns and strings #92812

alexwizp commented Feb 25, 2021 •

edited

Loading

ryankeairns left a comment •

edited

Loading

alexwizp commented Mar 25, 2021 •

edited

Loading

stratoula commented Mar 26, 2021 •

edited

Loading

VladLasitsa commented Mar 26, 2021 •

edited

Loading